-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Added CompilerServices.SkipLocalsInitAttribute #20093
Conversation
This will not be in C# 8.0. I think it's likely to come later though 8.1, or beyond. |
Thanks, @jaredpar. Is it definitely going to happen, and when it does, is this definitely the interface it'll use? With thanks to @lkts, I'm tempted to say we should close this and only bring it back when we're ready to pursue it in the compiler. |
I think it's about as certain as we can be about a language feature. The attribute has already gotten LDM approval hence it is fairly likely. But with all language features it's not shipped until it's shipped. My 2 cents: i'd hold off on merging this until we can get the resources together to commit to getting it into a C# release. |
I think it's time to re-open this, since we're committing SkipLocalsInit for the next compiler release. |
@agocke, does the given API match what the compiler will be adding for the next release? Is this something that we can track internally and reopen as soon as the compiler support is merged or is the compiler wanting the framework API before it starts the work (I think, normally, the compiler just supports manually declared attributes for these kinds of things, so I would guess we could wait for compiler support to be merged). |
This looks like the right surface area, to me. Notably, |
To be clear, |
Why wouldn't Assembly be supported by the compiler? Assuming we'd use this in corelib/corefx instead of illink, I think we'd want that. |
I can't really find why we did it, but it seems intentional. I think it might have been caution about net modules? If you put the attribute on an assembly and compile to a netmodule, then the attribute will get lifted to the assembly level when the netmodule is compiled, I think, but the linker that's including the netmodule may not respect the assembly level attribute. Is the module target that much different? Do you have multi-module assemblies? |
Shouldn't this also be allowed on interfaces, now that we have default interface methods?
I don't think we do in corefx, though I don't know for sure. |
I would imagine yes. There may actually be other attributes in the framework which are allowed on class/struct today that would also apply to default interface methods... Perhaps those should be audited? |
Yes, this makes sense. I will add test for it. |
This attribute supports a new compiler feature that allows a user to specify that they don't want to emit the CLR `.locals init` portion of the header in methods. This can sometimes produce a performance improvement but, in some cases, can reveal uninitialized memory to the application. Ported from dotnet/coreclr#20093
Contributes to https://github.com/dotnet/corefx/issues/29026.
I assume
IsEnabled
is not needed since there is no override behavior.@tannergooding